Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial suggestion minor tests completed sucessfully #183

Merged
merged 10 commits into from
Oct 8, 2024

Conversation

OscarSnowPlow
Copy link
Contributor

@OscarSnowPlow OscarSnowPlow commented Oct 3, 2024

Description

What type of PR is this?

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

https://snplow.atlassian.net/jira/software/c/projects/PE/boards/50?assignee=712020%3Ab916e641-2c6f-4de9-82f6-d175e2e15bee&selectedIssue=PE-6573

Updated the macro to stop an error message occurring when a specific scenario appears in the attribution model.

This change modifies the macro adding a new argument. The argument is a boolean true or false which is an optional argument which automatically sets to false. If I change the argument is set to false the macro runs as it did previously, but when set to true instead of outputting a high date time stamp ('9999-01-01','999-01-02) it outputs ('0000-01-01','0000-01-02'.

note.
there was an issue with testing because of a dbt update which required me seeking some help from Agnes to change the version of testing's dbt. without the dbt vewrsion issue the code passed the test

Checklist

  • πŸ’£ Is your change a breaking change?
  • πŸ“– I have updated the CHANGELOG.md

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ““ internal package docs (ymls, macros, readme, if applicable)
  • πŸ“• I have raised a Snowplow documentation PR if applicable (Link here if required)
  • πŸ™… no documentation needed

@OscarSnowPlow OscarSnowPlow self-assigned this Oct 3, 2024
@agnessnowplow agnessnowplow force-pushed the macro_update_PE-6573 branch 12 times, most recently from dfea5ec to 718d779 Compare October 4, 2024 14:12
@OscarSnowPlow OscarSnowPlow marked this pull request as ready for review October 7, 2024 13:27
@OscarSnowPlow OscarSnowPlow requested a review from a team as a code owner October 7, 2024 13:27
@OscarSnowPlow OscarSnowPlow requested review from ilias1111 and agnessnowplow and removed request for a team October 7, 2024 13:28
@OscarSnowPlow
Copy link
Contributor Author

I need to change the base branch from main to one of the release branches which one should this be?

@OscarSnowPlow OscarSnowPlow changed the base branch from main to release/snowplow-utils/0.17 October 8, 2024 08:37
macros/utils/return_limits_from_model.sql Outdated Show resolved Hide resolved
@OscarSnowPlow OscarSnowPlow merged commit 7536b51 into release/snowplow-utils/0.17 Oct 8, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants